[flink] Flink batch read based on SST merge. - #3677
Conversation
18edf1f to
a9044ac
Compare
a9044ac to
1496616
Compare
naivedogger
left a comment
naivedogger
left a comment
There was a problem hiding this comment.
Thanks @loserwang1024 for the PR! Overall looks good to me — left a few comments, PTAL.
| IOUtils.closeQuietly(logScanner); | ||
| } | ||
|
|
||
| private static ProjectionPlan createProjectionPlan( |
There was a problem hiding this comment.
nit: this is logically identical to LakeSnapshotAndLogSplitScanner#getNeedProjectFields(pk-column completion + adjustProjectedFields mapping, just packaged differently). Maybe worth extracting into a shared helper in this package so future fixes don't need to be applied twice?
| return true; | ||
| } | ||
|
|
||
| CloseableIterator<InternalRow> snapshotRows = snapshotScanner.pollBatch(timeout); |
There was a problem hiding this comment.
initializeSnapshotIterator calls snapshotScanner.pollBatch() only once and treats the returned iterator as the complete snapshot stream. This assumes a single pollBatch returns the whole snapshot, which holds for KvSnapshotBatchScanner today but violates the poll-until-null contract of BatchScanner — a chunked implementation would silently lose data. Could we make SnapshotRecordIterator lazily poll until pollBatch returns null? If the underlying scanner ever becomes chunked (like KvBatchScanner), this would silently drop snapshot data.
| import static org.apache.fluss.utils.Preconditions.checkArgument; | ||
|
|
||
| /** | ||
| * The hybrid split for first reading the snapshot files and then switch to read the cdc log from a |
There was a problem hiding this comment.
nit: the class javadoc still only describes the streaming two-phase behavior ("first reading the snapshot files and then switch to read the cdc log"). Since this split now also models bounded batch reads (merged snapshot + bounded log, possibly with no snapshot at all via NO_SNAPSHOT_ID), could we update the javadoc and add a doc for the NO_SNAPSHOT_ID semantics?
2 participants
Purpose
Linked issue: close http://github.com/apache/fluss/issues/3656
Brief change log
Tests
API and Format
Documentation